Draft: Use Certificates instead of CertificateRequests #55

jacksgt · 2024-02-13T09:39:39Z

As discussed in #49 , using CertificateRequest resources for interacting with cert-manager has several drawbacks:

this project (openshift-routes) has to duplicate logic for handling the creation of certificates that already exists in cert-manager.
observability is impaired because cert-manager does not offer metrics for CertificateRequest resources, only for Certificates.

Therefore, this patch replaces the management of CertificateRequests with Certificates (and Secrets). In return, it requires slightly higher privileges because openshift-routes needs to be able to create and read Certificates.
At the same time, the code complexity and LOC has been reduced.

As discussed in cert-manager#49 , using `CertificateRequest` resources for interacting with cert-manager has several drawbacks: - this project (openshift-routes) has to duplicate logic for handling the creation of certificates that already exists in cert-manager. - observability is impaired because cert-manager does not offer metrics for `CertificateRequest` resources, only for `Certificates`. Therefore, this patch replaces the management of `CertificateRequests` with `Certificates` (and `Secrets`). In return, it requires slightly higher privileges because openshift-routes needs to be able to create and read `Certificates`. At the same time, the code complexity and LOC has been reduced. Signed-off-by: Jack Henschel <[email protected]>

jetstack-bot · 2024-02-13T09:39:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign wallrj for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jacksgt · 2024-02-13T09:41:26Z

I have not touched the tests yet as I first want to invite some discussion about this proposal.
But I already gave it a try on my local cluster:

Create a local ClusterIssuer:

oc apply -f - <<EOF
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: example
  namespace: openshift-cern-cert-manager
spec:
  isCA: true
  privateKey:
    algorithm: ECDSA
    size: 256
  secretName: example
  commonName: example root
  duration: 262800h
  issuerRef:
    name: selfsigned
    kind: ClusterIssuer
---
apiVersion: cert-manager.io/v1
kind: ClusterIssuer
metadata:
  name: example
  namespace: openshift-cern-cert-manager
spec:
  ca:
    secretName: example
EOF

oc get clusterissuer
NAME               READY   AGE
example            True    32m

Create a Route with annotations for openshift-routes:

oc create -f - <<EOF
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  name: example-route
  annotations:
    cert-manager.io/issuer-name: example
    cert-manager.io/issuer-kind: ClusterIssuer
  namespace: default
spec:
  wildcardPolicy: None
  to:
    name: httpbin
EOF

Start openshift-routes locally:

go run internal/cmd/main.go --namespace openshift-cern-cert-manager --enable-leader-election=false
I0213 10:32:31.652525   30565 internal.go:360] cert-manager-openshift-routes/controller-manager "msg"="Starting server" "addr"={"IP":"::","Port":6060,"Zone":""} "kind"="health probe"
I0213 10:32:31.652683   30565 controller.go:177] cert-manager-openshift-routes/controller-manager "msg"="Starting EventSource" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route" "source"="kind source: *v1.Route"
I0213 10:32:31.652714   30565 server.go:50] cert-manager-openshift-routes/controller-manager "msg"="starting server" "addr"={"IP":"::","Port":9402,"Zone":""} "kind"="metrics" "path"="/metrics"
I0213 10:32:31.652789   30565 controller.go:177] cert-manager-openshift-routes/controller-manager "msg"="Starting EventSource" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route" "source"="kind source: *v1.Certificate"
I0213 10:32:31.652861   30565 controller.go:177] cert-manager-openshift-routes/controller-manager "msg"="Starting EventSource" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route" "source"="kind source: *v1.Secret"
I0213 10:32:31.652909   30565 controller.go:185] cert-manager-openshift-routes/controller-manager "msg"="Starting Controller" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route"
I0213 10:32:32.162140   30565 controller.go:219] cert-manager-openshift-routes/controller-manager "msg"="Starting workers" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route" "worker count"=1
  C-c C-cI0213 10:32:55.003628   30565 internal.go:581] cert-manager-openshift-routes/controller-manager "msg"="Stopping and waiting for non leader election runnables" 
I0213 10:32:55.003776   30565 server.go:43] cert-manager-openshift-routes/controller-manager "msg"="shutting down server" "addr"={"IP":"::","Port":9402,"Zone":""} "kind"="metrics" "path"="/metrics"
I0213 10:32:55.003897   30565 internal.go:585] cert-manager-openshift-routes/controller-manager "msg"="Stopping and waiting for leader election runnables" 
I0213 10:32:55.003968   30565 controller.go:239] cert-manager-openshift-routes/controller-manager "msg"="Shutdown signal received, waiting for all workers to finish" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route"
I0213 10:32:55.004010   30565 controller.go:241] cert-manager-openshift-routes/controller-manager "msg"="All workers finished" "controller"="route" "controllerGroup"="route.openshift.io" "controllerKind"="Route"
I0213 10:32:55.004043   30565 internal.go:591] cert-manager-openshift-routes/controller-manager "msg"="Stopping and waiting for caches" 
I0213 10:32:55.004332   30565 internal.go:595] cert-manager-openshift-routes/controller-manager "msg"="Stopping and waiting for webhooks" 
I0213 10:32:55.004391   30565 internal.go:599] cert-manager-openshift-routes/controller-manager "msg"="Wait completed, proceeding to shutdown the manager"

Check resources:

$ oc -n default get route example-route -o yaml
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  annotations:
    cert-manager.io/issuer-kind: ClusterIssuer
    cert-manager.io/issuer-name: example
    openshift.io/host.generated: "true"
  creationTimestamp: "2024-02-13T09:25:07Z"
  name: example-route
  namespace: default
  resourceVersion: "469150287"
  uid: d0600c4b-cb06-4fef-ba0b-73b1a0e10ee0
spec:
  host: example-route-default.example.com
  tls:
    certificate: |
      -----BEGIN CERTIFICATE-----
      MIICcjCCAhmgAwIBAgIRAOFkhOoNFxv59rrOVz7dkEswCgYIKoZIzj0EAwIwFzEV
      MBMGA1UEAxMMZXhhbXBsZSByb290MB4XDTI0MDIxMzA5Mjc0MVoXDTI0MDUxMzA5
      Mjc0MVowADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKJGBQ7ndoip
      nxY7/+8wT3rLrOHwMfaJ6m52dBVOBELW5rE3WBhrEfuMtISVJ8gfWn7VupT0LUi7
      pnYaFyf0TVf8DpSyL7OMtPKE/ZxFCKOrc5mQiSjWmwSEwQyMYS3cdvadDIzV7kbO
      2DFyvFcATt/yUVwyvNJsxr3oQ7pgQahJyp98RgAU6lCgJ7jfWyVGBinUFGx3yzfD
      fqK4wEKoHn27VKmHiQQB9TssQi9ZHE5Fm66wBCBUc6tIaQlOdpUnE/kHasnZ7LmC
      rITE0ARQcmKBq4dzeyhAV2VNBzDnMP0BOItObn4Sn7YT2/QTmU39tK7DzRxzsQY1
      xtpmL8Q1l0UCAwEAAaOBkTCBjjAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYI
      KwYBBQUHAwEwDAYDVR0TAQH/BAIwADAfBgNVHSMEGDAWgBQxTapkQk5YhD3LbNYc
      0obAFFo8gzA4BgNVHREBAf8ELjAsgipleGFtcGxlLXJvdXRlLWRlZmF1bHQuY2x1
      LWphY2stZGV2LmNlcm4uY2gwCgYIKoZIzj0EAwIDRwAwRAIgXmvW5M340E8eK0pX
      MNX+GnUQTs8qE2jdiPEUfGS2LvsCIAS/qFPaaKj4ARAQpT4Dd7IM2yVK6dDFFvms
      SpVAUfC6
      -----END CERTIFICATE-----
    insecureEdgeTerminationPolicy: Redirect
    key: |
      -----BEGIN RSA PRIVATE KEY-----
      MIIEowIBAAKCAQEAokYFDud2iKmfFjv/7zBPesus4fAx9onqbnZ0FU4EQtbmsTdY
      GGsR+4y0hJUnyB9aftW6lPQtSLumdhoXJ/RNV/wOlLIvs4y08oT9nEUIo6tzmZCJ
      KNabBITBDIxhLdx29p0MjNXuRs7YMXK8VwBO3/JRXDK80mzGvehDumBBqEnKn3xG
      ABTqUKAnuN9bJUYGKdQUbHfLN8N+orjAQqgefbtUqYeJBAH1OyxCL1kcTkWbrrAE
      IFRzq0hpCU52lScT+QdqydnsuYKshMTQBFByYoGrh3N7KEBXZU0HMOcw/QE4i05u
      fhKfthPb9BOZTf20rsPNHHOxBjXG2mYvxDWXRQIDAQABAoIBADWOAEtb7o3J1Twk
      TyIkgoaXQ5ZJjGO+PoV4SHVjixp4DCi+iC9+3q9zT3xWMYvldRtY9DwGng9cBuMB
      V1UTVpdME4/VgtKyBGHprD1vtxs1EXDD99Bniz+hhIjcp5HYKdbYG/U7AWmTCFzB
      bhEUg/N66IkSIakcxzaTug5/iAu+0DoMVvKhiLkCSH6QYmi16Fxic0eGu7OlMVHu
      2wmQmMnu/5ExOj4RRYIzSqm9y4fNyW2Z7hWg/9Gx+ATw+ZdulUhMxJHjGxvAGR+3
      6GXoQ2qUNaYvbe5x2yiQbQNShxfuMnwdSpX0VCXCz6C1z5w2S/osmMEWCB2W+smz
      wzcQGd0CgYEAznQRlAnJUHOxeugOUxKq3gdKHbqSEiEPzMcRboqhu4fU2g5tKq8m
      VsHBCp/2wIfgpGmfSOpX67EDVWgQBTTq8NYZ9hpKyAep3OkKXR1b9ILoW0A1Gz0c
      VVHGZv2jmoK0pSR3i1QeGq9RtHitwwNrik+VlM9c3hzZ9JzlZ0oSrNsCgYEAyTep
      CQQUGRfaBvXOKuSYkFjbO7v+R6kB60XU+IqvirwksdSIlG4fH+ITIqtS0I2mJ9cI
      tdGj4uezo8f1nCJp4vmelR7SLqmMJqPrHe8aKWbsgX/XoBUvO7lpa20660DYC7v3
      XmipwCttPQp9R8MrLbc+wW+zNcUdp4EdiiCX9l8CgYBdJp2vz+KXfDv+Gqor7WZP
      G7bjRwUVTPmWCdPhrode1+DAKnYzJigESRPSuW5aXHSNemK2QZY97/ZzGKrxznib
      Bd9c3WwUaPDJjhRxAwg0gMRaN9Q+YApirKz6V0L0OjlLsfKGWQPkQmp5JWIxdV+W
      XmY9aHqcdSQabJhNTGy0tQKBgQCwW3hrzodO9vjA4O+x6GlPGpIL6NkVNavY6Xuf
      2u3ASuZedki+z0W4TA05da8/2uamRHH96aAaX7my8q7yCbeEmAPF7x2IiFGuDD0m
      H0puvybK2aHDTM35Kqia30Gkr1Cr+DL3LASbyXQU6/yhyQ0vJEx8fco0dm9nQGMD
      jU2jQQKBgBqSwHhguULjemX42K/FUt2eFZzsNfuAvxiyKdC+HUcMWiiWb14dTnwZ
      Ftg0b/LaIyackfKH80dQ+sDquXOj94tH6knaDo4GZMjp0Rk8fkZqy97XOe4miaHE
      PJpxIVeJD2wX27I/lPHcGu2JHMJ+mI8Qn2+a5XZwWwX3vdSXrJ1i
      -----END RSA PRIVATE KEY-----
    termination: edge
  to:
    kind: Service
    name: httpbin
    weight: 100
  wildcardPolicy: None
status:
  ingress:
  - conditions:
    - lastTransitionTime: "2024-02-13T09:25:07Z"
      status: "True"
      type: Admitted
    host: example-route-default.example.com
    routerCanonicalHostname: router-apps-shard-1.example.com
    routerName: apps-shard-1
    wildcardPolicy: None

$ oc -n default get certificate
NAME                  READY   SECRET                   AGE
example-route-xs7vt   True    example-route-tls-cert   84s

$ oc -n default get secret | grep route
example-route-tls-cert     kubernetes.io/tls                     3      99s

TODOs:

how to handle deletion of secrets / secrets that already exist?
update tests (after first code review)
update RBAC in deployment manifests / Helm chart (after first code review)
edge cases?

ctrought · 2024-02-13T19:57:25Z

Looking forward to this!

It would be great if we could also attach multiple routes to the same Certificate, this makes most sense for path based routes or any specifying alt names via cert-manager.io/alt-names. Could have one of the routes being the manager for the certificate via annotations, and other routes specify the exact Certificate to use via an annotation. That would require the Certificate name to be predictable (e.g. match name of owner route), alternatively point directly to the secret (via annotation) similar to how ingress works today where the configured cert does not necessarily need to be configured via ingress annotations.

jacksgt · 2024-02-14T08:47:20Z

It would be great if we could also attach multiple routes to the same Certificate

I agree, this should be easily possible with an annotation like cert-manager.io/certificate-name, which would solve #54
This would also allow the use of wildcard certificates, as requested here: #13

But first this initial draft I didn't want to overcomplicate and introduce new features. :-)

jacksgt · 2024-02-26T12:24:42Z

Hi @maelvls , would you mind taking a look at this?

inteon · 2024-03-01T11:12:22Z

this project (openshift-routes) has to duplicate logic for handling the creation of certificates that already exists in cert-manager.

I think this is the case for multiple cert-manager projects: csi-driver, csi-driver-spiffe, istio-csr, ...
See https://cert-manager.io/docs/usage/ for an overview of the different ways CRs are created.

maelvls · 2024-03-07T16:00:43Z

I responded to Tim's comment in #49 (comment).

Just wondering, how much effort would it be able to use one or the other (either create a CR or create a Certificate)? @jacksgt

jacksgt · 2024-03-08T16:14:54Z

Just wondering, how much effort would it be able to use one or the other (either create a CR or create a Certificate)?

I think the initial implementation would not be an issue. In that case I'd suggest having two separate sync functions (one for each "mode").
However I'm not sure how maintainable and understandable this code will be over time ...

cert-manager-prow · 2024-08-22T12:17:46Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Use Certificates over CertificateRequests (#55 followup)

SgtCoDFish · 2024-10-03T12:18:40Z

Thanks so much for the initial work on this @jacksgt - really appreciate it and sorry it took a while to get it merged.

We've now merged #101 which got this merged. I preserved your original commit so you'll get credit for the contribution.

I hope you're able to test this out when we release it soon!

I'll close this now since it's merged.

jetstack-bot added the dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. label Feb 13, 2024

jetstack-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 13, 2024

maelvls mentioned this pull request Mar 1, 2024

Moving from CertificateRequests to Certificates: A Survey #49

Closed

maelvls mentioned this pull request Jul 18, 2024

openshift-routes should create Certificates instead of CertificateRequests #34

Closed

cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 22, 2024

This was referenced Sep 30, 2024

Use Certificates over CertificateRequests (#55 followup) #101

Merged

Add option to switch between using certificates and CRs #102

Closed

cert-manager-prow bot added a commit that referenced this pull request Oct 3, 2024

Merge pull request #101 from SgtCoDFish/jacksgt-use-certificates

e8a79cc

Use Certificates over CertificateRequests (#55 followup)

SgtCoDFish closed this Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Use Certificates instead of CertificateRequests #55

Draft: Use Certificates instead of CertificateRequests #55

jacksgt commented Feb 13, 2024 •

edited

Loading

jetstack-bot commented Feb 13, 2024

jacksgt commented Feb 13, 2024 •

edited

Loading

ctrought commented Feb 13, 2024 •

edited

Loading

jacksgt commented Feb 14, 2024

jacksgt commented Feb 26, 2024

inteon commented Mar 1, 2024

maelvls commented Mar 7, 2024 •

edited

Loading

jacksgt commented Mar 8, 2024

cert-manager-prow bot commented Aug 22, 2024

SgtCoDFish commented Oct 3, 2024

Draft: Use Certificates instead of CertificateRequests #55

Draft: Use Certificates instead of CertificateRequests #55

Conversation

jacksgt commented Feb 13, 2024 • edited Loading

jetstack-bot commented Feb 13, 2024

jacksgt commented Feb 13, 2024 • edited Loading

ctrought commented Feb 13, 2024 • edited Loading

jacksgt commented Feb 14, 2024

jacksgt commented Feb 26, 2024

inteon commented Mar 1, 2024

maelvls commented Mar 7, 2024 • edited Loading

jacksgt commented Mar 8, 2024

cert-manager-prow bot commented Aug 22, 2024

SgtCoDFish commented Oct 3, 2024

jacksgt commented Feb 13, 2024 •

edited

Loading

jacksgt commented Feb 13, 2024 •

edited

Loading

ctrought commented Feb 13, 2024 •

edited

Loading

maelvls commented Mar 7, 2024 •

edited

Loading